Skip to content

feat(aztec-nr): wire constrained message delivery#23866

Open
vezenovm wants to merge 65 commits into
merge-train/fairies-v5from
mv/f-669-constrained-delivery-helpers
Open

feat(aztec-nr): wire constrained message delivery#23866
vezenovm wants to merge 65 commits into
merge-train/fairies-v5from
mv/f-669-constrained-delivery-helpers

Conversation

@vezenovm

@vezenovm vezenovm commented Jun 4, 2026

Copy link
Copy Markdown
Contributor

Fixes F-669
Fixes F-670

The change

  • Adds constrained-delivery helpers that resolve or bootstrap the app-siloed handshake secret, seed same-tx index reuse, validate index 0 against the standard HandshakeRegistry, and constrain index > 0 through the previous chain nullifier.
  • Wires constrained private message delivery to derive constrained log tags from the resolved (secret, index) pair and emit the current constrained-message nullifier.
  • Keeps constrained logs from being squashed with note hashes so recipient discovery can advance the per-secret index chain.
  • Authorizes the standard HandshakeRegistry utility reads needed by the flow.
  • Adds TXE, snapshot, and e2e coverage for bootstrap, reuse, index advancement, missing prior-nullifier failure, invalid API combinations, and standard-registry constrained delivery.

Large portion of the diff is tests.

F-741 follow-up for the test skipped in fd07996.

Concurrency / batching constraint (worth a look)

Pinning the e2e tests surfaced a sequencing constraint in constrained delivery that reviewers should sanity-check:

  • Parallel sends on one (sender, recipient, secret) chain collide. Each send is keyed on an incrementing index, so two sends fired as separate parallel txs read the same index and one tx is rejected. Distinct recipients are distinct chains and parallelize fine. Pinned as it.failing (documents the limit; flips green if parallel sends ever become supported).
  • Same-chain sends can be batched into one tx, but only onto an already-committed handshake. A later send discharges its predecessor check against a same-tx pending nullifier. Works both via a single contract call (emit_two_events) and client-side BatchCall.
  • Batching onto a brand-new chain does not reuse, it re-handshakes. The reuse-vs-bootstrap decision in get_or_create_app_siloed_handshake_secret is a utility call reading committed state, so a bootstrap earlier in the same tx is invisible and the later send mints a fresh secret on a separate chain (next index lands at 1, not 2). A new recipient therefore needs one landed tx to establish the chain before sends can be batched onto it. This is why the batched tests seed the handshake first.

All four behaviors are pinned in e2e_constrained_delivery.test.ts; the rationale lives in the module doc and on get_or_create_app_siloed_handshake_secret. Follow-up: allowing utility functions to execute against combined committed + pending transaction state is tracked in F-238.

@vezenovm vezenovm force-pushed the mv/f-697-general-delivery-builder branch from b9f12b8 to befb4ea Compare June 5, 2026 01:34
@vezenovm vezenovm force-pushed the mv/f-669-constrained-delivery-helpers branch from 69abd69 to e2a43cd Compare June 5, 2026 01:34
vezenovm added a commit that referenced this pull request Jun 10, 2026
)

Fixes
[F-697](https://linear.app/aztec-labs/issue/F-697/aztec-nr-extend-the-onchaindelivery-builder-for-secret-origin)
Fixes
[F-649](https://linear.app/aztec-labs/issue/F-649/fixaztec-nr-skip-note-log-linkage-for-onchain-constrained-delivery-to)

## Summary

- Extends `MessageDelivery` with separate typed builders for
unconstrained and constrained on-chain delivery.
- Adds internal tag-secret derivation config:
  - offchain: none
- onchain unconstrained: address-pair by default, with optional
non-interactive handshake selection
  - onchain constrained: non-interactive handshake
- Introduces `OnchainDeliveryMode` as the typed on-chain mode used by
tagging-index helpers and the HandshakeRegistry ABI instead of raw `u8`
values.
- Keeps invalid delivery/tag-derivation combinations rejected at compile
time where possible.
- Stops linking constrained-tagged note logs to note-hash squashing, so
removing a squashed note does not break the recipient tag index chain.
- Re-pins the HandshakeRegistry standard contract and updates the
generated standard-contract metadata.

Constrained tagging is still not fully wired up: the delivery config is
validated, but tag emission remains mocked through the existing
wallet-derived unconstrained path behind TODO(#14565).

Stack: #23875, then this PR, then #23866, then #23867.

---------

Co-authored-by: AztecBot <tech@aztec-labs.com>
Co-authored-by: Nicolas Chamo <nicolas@chamo.com.ar>
Base automatically changed from mv/f-697-general-delivery-builder to merge-train/fairies-v5 June 10, 2026 15:51
@vezenovm vezenovm force-pushed the mv/f-669-constrained-delivery-helpers branch from 6337393 to c552943 Compare June 10, 2026 17:55
…et_and_index

Also corrects the safety comment on the registry utility call: a duplicate
handshake replaces the stored note rather than failing, so the actual
argument is that every branch constrains the secret it returns.
@vezenovm vezenovm changed the title feat(aztec-nr): add calculate_secret_and_index constrained-delivery helper feat(aztec-nr): add resolve_secret_and_index constrained-delivery helper Jun 10, 2026
@vezenovm vezenovm requested a review from nchamo June 11, 2026 14:19
@vezenovm vezenovm added the ci-no-fail-fast Sets NO_FAIL_FAST in the CI so the run is not aborted on the first failure label Jun 16, 2026
StatefulTest is a generic test contract used across e2e/PXE unit tests
that don't exercise constrained-delivery semantics. Routing its
`create_note`/`create_note_no_init_check`/`destroy_and_create_no_init_check`
inserts through `onchain_constrained()` ties those tests to the
constrained-msg nullifier chain — that breaks parallel-deploy block
tests (chain collision on shared recipient), PXE unit tests that don't
set up handshake state, and the e2e_2_pxes "no account secret key"
scenario. Dedicated constrained-delivery coverage lives in
ConstrainedDeliveryTest + e2e_constrained_delivery; same precedent as
the earlier state_vars test switch.
@AztecBot AztecBot force-pushed the mv/f-669-constrained-delivery-helpers branch from 7b401f0 to 7628eab Compare June 16, 2026 22:30
const deployedContract = await deployContract();

const sender = owner;
const insertConstrainedSelector = await deployedContract.methods.insert_note_constrained.selector();

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just highlighting this is specifically a constrained note log test

@vezenovm vezenovm marked this pull request as ready for review June 17, 2026 15:53
@vezenovm vezenovm requested a review from a team as a code owner June 17, 2026 15:53
@vezenovm vezenovm marked this pull request as draft June 17, 2026 16:05
@vezenovm vezenovm marked this pull request as ready for review June 17, 2026 16:41

@nchamo nchamo left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm still going over it, but wanted to share some initial feedback

return (
functionSelector.equals(await STANDARD_HANDSHAKE_REGISTRY_GET_APP_SILOED_SECRET_SELECTOR) &&
args.length >= 4 &&
// TODO(F-671): will be replaced with `self.msg_sender()` once utility context exposes it.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While it does make sense to have this, since we already have a PR in place (#24062), I would avoid it, simply to avoid merging conflicts

@vezenovm vezenovm Jun 17, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would avoid it, simply to avoid merging conflicts

Just for clarity sake, you mean this entire check right?

Comment thread noir-projects/protocol-fuzzer/contracts/side_effect_contract/src/main.nr Outdated
Comment thread noir-projects/noir-contracts/contracts/test/test_log_contract/src/main.nr Outdated
Comment thread noir-projects/noir-contracts/contracts/test/state_vars_contract/src/main.nr Outdated
/// The fields are private and there is no public constructor: a `MessageDelivery` can only be produced by a
/// [`MessageDeliveryBuilder`] that enforces valid configurations, so invalid field combinations cannot be
/// represented to the consumer.
/// [`MessageDeliveryBuilder`]. The delivery APIs validate the built configuration before consuming it.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know that we are currently allowing to create "invalid" states, but that is only temporary (for a few PRs), right? The end state will only allow devs to build valid states, right?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes that will be the end state.

Comment thread noir-projects/aztec-nr/aztec/src/messages/delivery/mode.nr Outdated
Comment thread noir-projects/aztec-nr/aztec/src/messages/delivery/constrained_delivery.nr Outdated
Comment thread noir-projects/aztec-nr/aztec/src/messages/delivery/constrained_delivery.nr Outdated
)
}

mod test {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it me, or are we missing quite a lot of tests here?

I think we have a lot of behaviors and decision trees on this file that we are not testing. Could we?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Most of the coverage is contained in constrained_delivery_test_contract/src/test.nr but yeah this could do with some unit tests.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added some unit tests

@AztecBot

Copy link
Copy Markdown
Collaborator

Flakey Tests

🤖 says: This CI run detected 1 tests that failed, but were tolerated due to a .test_patterns.yml entry.

\033FLAKED\033 (8;;http://ci.aztec-labs.com/bc2b942422d9df29�bc2b942422d9df298;;�): yarn-project/end-to-end/scripts/run_test.sh ha src/composed/ha/e2e_ha_full.parallel.test.ts "should not be affected by process.env.TZ changes" (44s) (code: 0)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci-draft Run CI on draft PRs. ci-no-fail-fast Sets NO_FAIL_FAST in the CI so the run is not aborted on the first failure claudebox Owned by claudebox. it can push to this PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants